Skip to content

test(Lightbox): the busy zoom proof judges the photo's top edge - #10648

Merged
cixzhang merged 2 commits into
meta:mainfrom
vjeux:test/lightbox-zoom-busy-top-edge
Nov 8, 2026
Merged

cixzhang merged 2 commits into
meta:mainfrom
vjeux:test/lightbox-zoom-busy-top-edge

Conversation

@vjeux

@vjeux vjeux commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

The double-tap zoom's busy proof reads the photo's top edge and spells out its per-frame check: vjeux/astryx@619ffeb...569ba49

Maintainer impact

"the double-tap zoom keeps advancing every frame while the main thread is blocked" fails in Chromium with "moved on 94 of the 95 frames the control moved": on #10622, #10571 and #10588 (line 990, before the paging).

Intended invariant

The photo still moves on every frame the compositor control moves, on the same four-frame floor; a main-thread or frozen zoom still fails. The stall rule is stricter: the sub-step run allowance no longer applies.

Change

The proof reads the top edge and stops 24 px from rest, where the edge's move between two cast frames falls under one cast step. At tempo 5 a 60 Hz frame moves it about 0.042 of what remains, so Chromium's 1 px casts of every frame fall under a step from 23.9 px. Blocked, WebKit casts every 116 ms or more in 2 px steps, so only from about 8 px; its judged part is unchanged.

Evidence

The four conditions:

  1. Cause: the height sums two whole-pixel edge reads, so near the end it can stall a frame while growing almost two, which the one-coordinate step rules took for a stall. One edge steps on every presented frame.
  2. Not weaker: the test asserts it outright: a new top-edge position on every frame the control moves, none excused as under a cast step, on the unchanged four-frame floor and stall bound.
  3. Red-first. Failure before the change: on main 372f681, in the CI profile (a 4-CPU container) plus six busy threads, Chromium fails 13 of 20. Success after the change: 27 of 28, WebKit 12 of 12 (8 of 8 before); idle, 6 of 6 each. The frozen-zoom and main-thread-zoom mutants still fail: 4 of 4 per engine, 2 of 2 loaded.
  4. feat(Lightbox): swipe between gallery photos like Photos #8197: cleared. Loaded, the proof fails 16 of 20 at its parent, 46d1813, and 17 of 20 at its merge, 88ba279.
  • Product/runtime behavior verified unchanged: test-only.
  • Still open: the 1 failure in 28 is not the old plateau but a one-frame hand-off lag at the zoom's start. The edge holds 220 px from rest for a frame, 64 to 70 ms into the block, while the control moves, then catches up 5 px. It came at six busy threads, heavier than CI, and is its own row for a later minProgressMs fix. Two replayed runs show such lags.

Scope

  • No intended public API, product behavior, visual direction, or policy change.
  • Product changes discovered during the work were removed or split.
  • Public text and artifacts contain no internal Meta context.

Testing

Core tsc, ESLint and Prettier are clean. The runs use a 4-CPU container (--cpus=4), with six spinning threads beside the browser for the loaded profile, and one Storybook build of main 372f681 for both arms. A replay feeds each run's recorded frames (the photo's top edge and height, the control's position) to the harness's busyWindowProof. From the height it reproduces all 50 recorded Chromium verdicts. From the top edge, CI's failing run and 38 of 40 loaded runs pass, and its 16 recorded WebKit verdicts are unchanged.

The double-tap zoom's busy proof read the photo's height. The height
adds two edges' whole-pixel steps, so near the end a frame can show no
new height while each edge still moves almost a pixel. The proof's
step rules read one coordinate's travel, so they took that plateau for
a stall. Reading the top edge gives them its own travel. The proof
stops 24 px of the edge from rest, where its move between two cast
frames falls under one cast step: Chromium casts every frame in 1 px
steps, and blocked WebKit casts every 116 ms or more in 2 px steps.
@vercel

vercel Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 8, 2026 7:38am UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 8, 2026
@astracat-bot

astracat-bot Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 8, 2026, 7:50 AM UTC

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Current run: The current CI run concluded skipped. Current analysis is unavailable.


Generated by PR Enrichment workflow | View current CI run

Besides the verdict, the double-tap zoom's busy proof now asserts what
it judges: the photo's top edge takes a new position on every frame the
control moves, with no frame excused as under a cast step, and the
control moves on at least the verdict's floor of four frames.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI has not run on this head yet, because the pull request is a draft on a fork branch. Once it is marked ready for review, a maintainer adds the ci:run label so CI runs the interaction proofs in Chromium and WebKit.

Judging the photo's top edge with a 24 px settle bound keeps the proof's intent. The photo must still move on every frame the compositor control moves, and the main-thread and frozen controls still fail. The new bound stops judging before the first frame where whole-pixel rounding holds the edge still, so those holds no longer read as stalls.

[Automated review]

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI has not run on this head yet, because the pull request is a draft on a fork branch. Once it is marked ready for review, a maintainer adds the ci:run label (it takes effect on the next push) so CI runs the interaction proofs in Chromium and WebKit.

The spelled-out assertions keep the proof's intent: the photo must take a new position on every frame the compositor control moves, and the main-thread and frozen controls still fail.

  • Miss: a non-blocking fix, since near the end this zoom moves about 0.032 of what remains each frame, not 0.042, so Chromium's per-frame move drops under 1 px about 30.5 px from rest rather than 23.9 px (comment above ZOOM_SETTLED_PX), which leaves the new belowStep assertion one frame of slack before a held frame enters the judged part; a 32 px bound gives it real margin (measured from the photo's rest, it also stops depending on when the screencast stops), and the comment's numbers need correcting either way.

[Automated review]

Comment on lines +1158 to +1160
* step. Near the end, each 60 Hz frame moves the spring about 0.042 of what
* remains (2π / 2.5 s, critically damped). Chromium casts every frame in 1 px
* steps, so its moves fall under a step 23.9 px from the end. Blocked, WebKit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking. For a critically damped spring from rest, what remains is A(1 + ωt)e^(-ωt), so a frame moves ωΔt · ωt/(1 + ωt) of it. That is about 0.032 at 24 px, not 0.042, which is the rate only as t grows without bound. The recorded top edge fits that curve to within 5 px in total. The move drops under 1 px at about 30.5 px from rest. The last ~4 judged frames move 0.86–0.98 px. In every run here, rounding still shows a new position on each of them, and the first held frame (y = 25) comes one frame after the cut. With expect(proof.control?.belowStep).toBe(0) that one frame is the whole margin. Replaying the same runs with the cast ending at rest (y = 0) fails the new assertion while the verdict passes. A 32 px bound, or settledAt read at rest, keeps belowStep 0 and still judges about 1.3 s.

@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 8, 2026
@vjeux
vjeux marked this pull request as ready for review November 8, 2026 08:30
@cixzhang
cixzhang merged commit 6744655 into meta:main Nov 8, 2026
129 checks passed
@vjeux
vjeux deleted the test/lightbox-zoom-busy-top-edge branch November 8, 2026 08:49

This branch was successfully deployed

1 active deployment
Preview — 569ba496 Deployed Nov 8, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants